-
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sync CRDTs with lexbox during crdt merge #1206
Conversation
C# Unit Tests75 tests 75 ✅ 5s ⏱️ Results for commit 5799ad6. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just have a handful of ideas and such 👍
# Conflicts: # backend/FwHeadless/SendReceiveHelpers.cs # backend/FwHeadless/appsettings.Development.json # backend/FwHeadless/appsettings.json # backend/FwLite/LcmCrdt/LcmCrdtKernel.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've left all my cents and this looks good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been wanting to change the API endpoint from /sync
to /api/crdt-sync
(or /api/crdtSync
or /api/crdtsync
) for a while now, and this seems like the right PR to do it in. Other than that it looks good.
previously crdt changes would only stay on the fw-headless server, now they are synced to lexbox. We're calling the API as that allows the clients to be notified directly, if we inserted directly into the db then clients would not be notified.
I also tweaked the sync api so it closes the fwdata project once it's done
closes #1172